Iterate object tracing #1882
Iterate object tracing #1882
Conversation
pypeit/par/pypeitpar.py
Outdated
| @@ -4206,6 +4206,11 @@ def __init__(self, trace_npoly=None, snr_thresh=None, find_trim_edge=None, | |||
| dtypes['find_maxdev'] = [int, float] | |||
| descr['find_maxdev'] = 'Maximum deviation of pixels from polynomial fit to trace used to reject bad pixels in trace fitting.' | |||
|
|
|||
| defaults['find_numiterfit'] = 10 | |||
There was a problem hiding this comment.
I'm wary of changing the default behavior for all datasets. Is that justified? Can we instead set this to 1 (or whatever the equivalent is to recover the current behavior) and then advertise that this option now exists and how it can improve traces? That will give us some time to decide if we want to change the default behavior.
There was a problem hiding this comment.
Yes, I think that's a fair point 👍 In principle, a good trace should simply break out of the loop during the 2nd iteration, since the only thing that changes is the initial guess. If the initial guess is close to the final position, nothing should change in the second iteration for a good fit.
|
Tests pass... |
There was a problem hiding this comment.
Hi @rcooke-ast I don't agree with this PR.
-
The fit_trace function that is being called is already performing iterations. So now you are just performing iterations on top of iterations in a double loop. There are already maxdev criteria implemented in that routine as well, and now you apply them again. This outer loop complicates the code with no added value or functionality, since all that functionality is already provided in fit_trace. It is possible that more iterations are needed or better trace converge criteria are needed, but those should be implemented in fit_trace not objs_in slit.
-
There are detailed QA options to show these fits and how they are progressing in fit_trace, by enabling the debug option. None of those plots are being shown. Instead you just show a screenshot of a bright object which does not provide any insight on the source of the problem. Furthermore, those traces look identical by eye, which is why we developed the detailed QA in fit_trace.
-
I'm not following your point about the slit edges not being known, but it suggests that the issue with the slit edges and not the object tracing. Nevertheless, we want the tracing to be robust against such cases. But I suspect the issue here is that your slits appear to be at a detector boundary. In that case, slit edges can end up being vertical lines, which is bad for object tracing. We have built in functionality already to make sure that the slit edge at the boundary is replaced by a curve with the shape of the well traced boundary, to deal with this problem. It could be something is not working properly there, so pasting on extra code in object tracing is not a good solution to such a problem if that is indeed the issue.
Take a look at my suggestions and let's then diagnose the problem properly and figure out how to make the code more robust if there are still issues.
|
Thanks for your thoughts @jhennawi. When debug=True, here is the QA (response to your point 2): 
When you say "performing iterations", do you mean that it uses a first guess, and then re-calculates the trace based on a better initial guess? This is the only repeated calculation that is performed. I'm proposing that we iterate over the second step, if needs be.
You are correct, the detector edges are not known, and are simply vertical lines. Also, this is the standard star trace, so we cannot use that as a crutch. |
|
No fit_trace does iterative fitting already with an initial guess/crutch that is dynamically updated. Let's discuss on Slack. |
# Conflicts: # doc/releases/1.17.2dev.rst
|
Thanks @jhennawi for the suggestion. I've now implemented the number of iterations as a free parameter, instead of being hard-coded. This produces equally good results. |
There was a problem hiding this comment.
Thanks @rcooke-ast. A very minor change requested related than orphan line. But otherwise approved.
| @@ -1904,17 +1911,18 @@ def objs_in_slit(image, ivar, thismask, slit_left, slit_righ, | |||
| msgs.info("Automatic finding routine found {0:d} objects".format(len(sobjs))) | |||
|
|
|||
| # Fit the object traces | |||
| tolerance = 0.1 # Tolerance in pixels before the trace fit has converged | |||
There was a problem hiding this comment.
This line should be removed right, tolerance since it is no longer used?
A user reported that the object traces are sometimes bad. I looked into this, and it appears to occur when the slit edges are not known, and the initial trace profile is no good. Successively iterating over the trace profile improve the result considerably. See screenshots below of the before (left) and after (right) when the masking near the bottom of the image disappears in the new version. Running dev suite now.